packages/withdrawals-contract/src/lib.rs (2)

48-59: LGTM: Implementation of fmt::Display for WithdrawalStatus.

The implementation is correct and improves the usability of the WithdrawalStatus enum by providing human-readable string representations. All enum variants are covered, ensuring exhaustiveness.

For consistency with the enum variant names, consider using uppercase strings in the match arms:

 impl fmt::Display for WithdrawalStatus {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let status_str = match self {
-            WithdrawalStatus::QUEUED => "Queued",
-            WithdrawalStatus::POOLED => "Pooled",
-            WithdrawalStatus::BROADCASTED => "Broadcasted",
-            WithdrawalStatus::COMPLETE => "Complete",
-            WithdrawalStatus::EXPIRED => "Expired",
+            WithdrawalStatus::QUEUED => "QUEUED",
+            WithdrawalStatus::POOLED => "POOLED",
+            WithdrawalStatus::BROADCASTED => "BROADCASTED",
+            WithdrawalStatus::COMPLETE => "COMPLETE",
+            WithdrawalStatus::EXPIRED => "EXPIRED",
         };
         write!(f, "{}", status_str)
     }
 }

This change would maintain consistency with the enum variant names and potentially make it easier to match string representations back to enum variants if needed.

---

7-7: Summary: Improved WithdrawalStatus enum usability.

The changes in this file are minimal and focused, aligning well with the PR objectives of making small SDK improvements. The implementation of fmt::Display for WithdrawalStatus enhances the usability of the enum by providing human-readable string representations. This change does not introduce any breaking changes and should improve the developer experience when working with withdrawal statuses.

Consider adding unit tests for the new fmt::Display implementation to ensure correct functionality and prevent regressions in future updates.

Also applies to: 48-59

packages/rs-dpp/src/core_types/validator_set/mod.rs (1)

38-44: LGTM! Consider future-proofing the implementation.

The Display implementation for ValidatorSet is correct and follows Rust's idiomatic patterns. It properly delegates the formatting to the contained ValidatorSetV0 instance.

For future-proofing, consider adding a catch-all arm to the match expression. This will ensure that the code continues to compile if new variants are added to the ValidatorSet enum in the future. Here's a suggested improvement:

impl Display for ValidatorSet {
    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
        match self {
            ValidatorSet::V0(v0) => write!(f, "{}", v0),
            _ => write!(f, "Unknown ValidatorSet variant"),
        }
    }
}

This change will make the code more resilient to future updates of the ValidatorSet enum.

packages/rs-sdk/src/mock/requests.rs (1)

170-191: LGTM! Consider using the global BINCODE_CONFIG for consistency.

The implementation of MockResponse for Element looks good and follows the pattern established for other types in this file. It correctly implements both mock_serialize and mock_deserialize methods using bincode.

For consistency with other implementations in this file, consider using the global BINCODE_CONFIG instead of creating a new standard configuration in each method. Here's a suggested modification:

 impl MockResponse for Element {
     fn mock_serialize(&self, _sdk: &MockDashPlatformSdk) -> Vec<u8> {
-        // Create a bincode configuration
-        let config = standard();
-
-        // Serialize using the specified configuration
-        bincode::encode_to_vec(self, config).expect("Failed to serialize Element")
+        bincode::encode_to_vec(self, BINCODE_CONFIG).expect("Failed to serialize Element")
     }

     fn mock_deserialize(_sdk: &MockDashPlatformSdk, buf: &[u8]) -> Self
     where
         Self: Sized,
     {
-        // Create a bincode configuration
-        let config = standard();
-
-        // Deserialize using the specified configuration
-        bincode::decode_from_slice(buf, config)
+        bincode::decode_from_slice(buf, BINCODE_CONFIG)
             .expect("Failed to deserialize Element")
             .0
     }
 }

This change would make the implementation more consistent with other MockResponse implementations in the file and reduce redundancy.

packages/rs-dpp/src/core_types/validator_set/v0/mod.rs (1)

40-67: LGTM: Well-implemented Display trait with a minor suggestion.

The Display implementation for ValidatorSetV0 is well-structured and provides a clear, human-readable representation of the struct. It correctly handles all fields, including the optional quorum_index.

For consistency, consider using the to_string() method for quorum_hash and threshold_public_key instead of hex::encode(), similar to how it's done in the Debug implementation. This would make the code more uniform across different trait implementations. For example:

write!(
    f,
    "ValidatorSet {{
    quorum_hash: {},
    // ... other fields ...
    threshold_public_key: {}
}}",
    self.quorum_hash.to_string(),
    // ... other fields ...
    self.threshold_public_key.to_string()
)

This change would maintain consistency with the Debug implementation while still providing a readable output.

packages/rs-sdk/src/platform/query.rs (1)

626-642: Implementation looks good, with a minor suggestion for improvement.

The new implementation of Query for NoParamQuery is correct and follows the established patterns in the file. However, consider replacing the unimplemented! macro with a proper Error return for better error handling when prove is true.

Consider replacing the unimplemented! macro with an Error return:

 fn query(self, prove: bool) -> Result<GetCurrentQuorumsInfoRequest, Error> {
     if prove {
-        unimplemented!(
-            "query with proof are not supported yet for GetCurrentQuorumsInfoRequest"
-        );
+        return Err(Error::Generic("Query with proof is not supported yet for GetCurrentQuorumsInfoRequest".into()));
     }

     let request: GetCurrentQuorumsInfoRequest = GetCurrentQuorumsInfoRequest {
         version: Some(get_current_quorums_info_request::Version::V0(
             GetCurrentQuorumsInfoRequestV0 {},
         )),
     };

     Ok(request)
 }

This change would provide a more graceful error handling approach and avoid potential panics in production code.

packages/rs-sdk/src/platform/fetch_many.rs (2)

19-20: Missing documentation for the new request type GetPathElementsRequest.

The newly added GetPathElementsRequest in the import statements lacks accompanying documentation. Providing documentation will enhance code readability and help other developers understand its purpose.

---

425-431: Add documentation for the new FetchMany implementation for Element.

The implementation of FetchMany<Key, Elements> for Element lacks documentation. To maintain consistency with other trait implementations and assist developers in understanding how to use this functionality, please add comprehensive documentation, including the purpose, usage examples, and supported query types.

Here's a suggested addition:

/// Fetch multiple elements from Platform.
///
/// Returns [Elements](drive_proof_verifier::types::Elements) indexed by their [Key](drive::grovedb::query_result_type::Key).
///
/// ## Supported query types
///
/// * [KeysInPath] - Specifies the path and keys from which to fetch elements.
///
/// ## Example
///
/// ```rust
/// use dash_sdk::platform::{FetchMany, KeysInPath};
/// use dash_sdk::Sdk;
/// use drive::grovedb::Element;
///
/// #[tokio::main]
/// async fn main() -> Result<(), dash_sdk::error::Error> {
///     let sdk = Sdk::new();
///     let path = vec!["some", "path"];
///     let keys = vec![b"key1".to_vec(), b"key2".to_vec()];
///     let query = KeysInPath::new(path, keys);
///     let elements = Element::fetch_many(&sdk, query).await?;
///     Ok(())
/// }
/// ```

Configuration used: CodeRabbit UI
Review profile: CHILL